Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking: size_t indices #39371

Closed
wants to merge 3 commits into from

Conversation

NickGerleman
Copy link
Contributor

Summary:
Yoga's public API exposes indices most often as uint32_t, with exception of clone callbacks which are int32_t. Yoga internally represents these indices as size_t when dealing with the child vector, and this is the true index.

This changes the API to consistently be size_t. This should not be breaking for most users, but will cause breaks where:

  1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
  2. Callers of YGNodeGetChildCount() are assigning to an int with less width than size_t and have strong warnings enabled.
  3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Differential Revision: D49130914

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner labels Sep 10, 2023
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49130914

NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 10, 2023
Summary:
Pull Request resolved: facebook#1366

X-link: facebook/react-native#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Differential Revision: D49130914

fbshipit-source-id: 202ed16597b34a804d771b161a6e2319f10dd4c3
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49130914

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 10, 2023
Summary:
X-link: facebook/yoga#1366

Pull Request resolved: facebook#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Differential Revision: D49130914

fbshipit-source-id: 24cc3f70bf7e3662f02adb5abac90d19a34c6344
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 10, 2023
Summary:
Pull Request resolved: facebook#1366

X-link: facebook/react-native#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Differential Revision: D49130914

fbshipit-source-id: e55ba3f0b59b8b00433da9d7ab898147c8cbcd0f
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49130914

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 10, 2023
Summary:
X-link: facebook/yoga#1366

Pull Request resolved: facebook#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Differential Revision: D49130914

fbshipit-source-id: 2a875a2c063ac8595e787fa0fe4f053bc50a4e74
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 10, 2023
Summary:
Pull Request resolved: facebook#1366

X-link: facebook/react-native#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Differential Revision: D49130914

fbshipit-source-id: 340555195d4b3d1f7179be99e7d6b49f3d22ec0b
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49130914

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 10, 2023
Summary:
X-link: facebook/yoga#1366

Pull Request resolved: facebook#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Differential Revision: D49130914

fbshipit-source-id: dcd1f2b6cf4a66c5b518cbe5be0bd9d7833729ca
@analysis-bot
Copy link

analysis-bot commented Sep 10, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,966,809 +382
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 9,559,402 +393
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: a2fb46e
Branch: main

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49130914

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 10, 2023
Summary:
X-link: facebook/yoga#1366

Pull Request resolved: facebook#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Differential Revision: D49130914

fbshipit-source-id: fb477f98443055ee5523521f562e37caffc25ba0
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 10, 2023
Summary:
Pull Request resolved: facebook#1366

X-link: facebook/react-native#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Differential Revision: D49130914

fbshipit-source-id: 6d929cf3757dc1cb08dba788cfbe05f4d0310980
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 11, 2023
Summary:
Pull Request resolved: facebook#1366

X-link: facebook/react-native#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D49130914

fbshipit-source-id: 6b9de766af3a9ccdc8772f60a1671c31d934ae91
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49130914

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 11, 2023
Summary:
X-link: facebook/yoga#1366

Pull Request resolved: facebook#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D49130914

fbshipit-source-id: 2c5cc30b8570b2df3a84091748d3144ff63a707e
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 12, 2023
Summary:
Pull Request resolved: facebook#1366

X-link: facebook/react-native#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D49130914

fbshipit-source-id: 6a9ad2385c8022c313e54fe8aaa5547bf76c9d49
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49130914

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 12, 2023
Summary:
X-link: facebook/yoga#1366

Pull Request resolved: facebook#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D49130914

fbshipit-source-id: 2dfd505a3f2bdd17b15cc46dc732fb55fc2d63e3
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 12, 2023
Summary:
Pull Request resolved: facebook#1366

X-link: facebook/react-native#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D49130914

fbshipit-source-id: bfaff0ed8c65e6d0b0bc9dffc18f10c177220529
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49130914

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 12, 2023
Summary:
X-link: facebook/yoga#1366

Pull Request resolved: facebook#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D49130914

fbshipit-source-id: 6fd72245cd8de59159a254dd98690c9dc2d81af0
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49130914

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 12, 2023
Summary:
X-link: facebook/yoga#1366

Pull Request resolved: facebook#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D49130914

fbshipit-source-id: 766d3ae999327169f89157fb6c5b484eb14d1aa6
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 12, 2023
Summary:
Pull Request resolved: facebook#1366

X-link: facebook/react-native#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D49130914

fbshipit-source-id: e3112b50a5a3202c6ecf256ce19508acad451c6d
NickGerleman and others added 3 commits September 11, 2023 19:00
Summary:
This changes public Yoga API to in more places accept const structures where before they required mutable ones.

This tries to avoid more breaking changes yet, e.g. changing callbacks to require clients do not modify nodes when they are passed for logging. We also don't have const variants for returning child structures which would allow mutation of dependencies of the const object. These would need new names under the public API, since we do not have operator overloading in C.

Differential Revision: D49130412

fbshipit-source-id: 426c88a15b32e4c2b38b728f69d1797de6347de3
Summary:
X-link: facebook/yoga#1369

Pull Request resolved: facebook#39370

This fixes const-correctness of callbacks (e.g. not letting a logger function modify nodes during layout). This helps us to continue to fix const-correctness issues inside of Yoga.

This change is breaking to the public API, since it requires a change in signature passed to Yoga.

Changelog: [Internal]

Differential Revision: https://internalfb.com/D49130714

fbshipit-source-id: 36247b2515c85fce88ef6db0241e960ade8724f1
Summary:
X-link: facebook/yoga#1366

Pull Request resolved: facebook#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D49130914

fbshipit-source-id: 5065cd77955bf475ea4ce1d401637ebe19778d30
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 12, 2023
Summary:
Pull Request resolved: facebook#1366

X-link: facebook/react-native#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D49130914

fbshipit-source-id: 55dd5310895f1764da42f1b870fad41b491b55d1
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49130914

facebook-github-bot pushed a commit to facebook/litho that referenced this pull request Sep 12, 2023
Summary:
X-link: facebook/yoga#1366

X-link: facebook/react-native#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D49130914

fbshipit-source-id: 6a004c160c4c50f68047b108508fd437156f5fac
facebook-github-bot pushed a commit to facebook/yoga that referenced this pull request Sep 12, 2023
Summary:
Pull Request resolved: #1366

X-link: facebook/react-native#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D49130914

fbshipit-source-id: 6a004c160c4c50f68047b108508fd437156f5fac
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in a37bd76.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants